fix(components): Fix autocomplete crashing on resize larger [JOB-151844]#2924
fix(components): Fix autocomplete crashing on resize larger [JOB-151844]#2924Aiden-Brine merged 7 commits intomasterfrom
Conversation
Deploying atlantis with
|
| Latest commit: |
052e989
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e688c724.atlantis.pages.dev |
| Branch Preview URL: | https://job-151844-max-depth-error.atlantis.pages.dev |
0087dfe to
241f2e8
Compare
| [], | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
my first thought is if we could use useCallbackRef instead.
something like const setReferenceElement = useCallbackRef(() => refs.setReference);
I'll have to double check this is equivalent.
There was a problem hiding this comment.
useCallbackRef(() => refs.setReference) isn’t equivalent because it returns the function instead of invoking it with the element. I’d need useCallbackRef((el) => refs.setReference(el)), but for a ref callback useStableSetReference is safer since it keeps refs current during render rather than after commit (useEffect).
There was a problem hiding this comment.
fair enough, I'm happy to leave it as is then.
| }); | ||
| Object.assign(elements.floating.style, { | ||
| maxHeight: `${maxHeight}px`, | ||
| width: `${rects.reference.width}px`, |
There was a problem hiding this comment.
clever! I forgot rects is available here. I like the colocation of dimension related setting code.
There was a problem hiding this comment.
I did notice we used to have a maxWidth too. I can't recall why that was added... maybe it's not necessary with this setup.
There was a problem hiding this comment.
maxWidth came from the old menuWidth workaround, and since we now set width from rects.reference.width in the floating size middleware (with the wrapper as the position reference), it seems redundant unless we intentionally want a different cap behavior.
|
|
||
| const mergedRef = useMemo( | ||
| () => mergeRefs([inputRef, inputTextRef]), | ||
| [inputRef, inputTextRef], |
There was a problem hiding this comment.
are these deps going to do anything? they're refs AKA objects, so React will just look at them and say "yup" that's the same object we had before. it won't look at the .current value.
There was a problem hiding this comment.
I realized the deps aren't needed and removed them in e34943b
|
I just merged the multiselect PR for Autocomplete which has created a couple merge conflicts 😅 the snapshots should be quick and easy. hopefully the main |
|
ahh I see what it is. my multiple work brought back the thing you removed but it doesn't show as a merge conflict because it's in a new file! in that gets us back to your changes in the |


Motivations
Maximum depth errors popped up in DataDog related to Autocomplete. It was very difficult to re-create but we eventually figured out it could be recreated sometimes, for certain people, with a monitor connected, if they had the autocomplete menu open, and they expanded their screen size from smaller to larger. Here is a stack trace:
Changes
Fix for the crash
mergeRefs([...])called inline creates a brand new callback function on every render of InputText. React tracks ref props by identity, if the ref prop is a different function than last render, React detaches the old one (calls it with null) then re-attaches the new one (calls it with the element). That detach chain ultimately callsrefs.setReference(null)inside floating-ui, which calls setState. When this happened during floating-ui's flushSync call on window resize, it caused a nested synchronous React commit that exceeded the maximum update depth limit and crashed.inputRef (our mergedInputRef from Autocomplete) and inputTextRef (an internal useRef, always stable) are both stable references. Memoizing with those as deps means mergedRef never changes identity, and React never schedules a detach/re-attach.
Upon fixing this though there were styling issues instead of the crash:
Fix for styling issues
Stable setReferenceElement
The refs object returned by useFloating can get a new object identity when floating-ui's internal state changes (e.g. after a position recalculation on resize). Since setReferenceElement listed refs as a dependency, it would get a new function identity whenever refs changed. That propagated up: referenceInputRef in Autocomplete.rebuilt.tsx depends on setReferenceElement, so it recreated too. Then mergedInputRef (which depends on referenceInputRef) recreated too. React then scheduled a detach/re-attach of the input ref, the same crash path described above.
The latestRefs pattern solves this by storing refs in a mutable useRef container that is updated synchronously every render. The setReferenceElement callback reads from the container at call time rather than capturing refs at creation time. Since a useRef container itself is always the same object identity, the dependency array can be empty, giving setReferenceElement a permanently stable identity.
sizemiddleware sets widthPreviously, the dropdown width was managed as React state (menuWidth), captured once when the input ref attached on mount via clientWidth. Because the above fixes made the ref stable, it no longer detaches and re-attaches on resize, which meant menuWidth was never refreshed after mount, the dropdown would show with a stale width after a screen resize.
The size middleware's apply callback runs on every autoUpdate trigger, so rects.reference.width is always the live, freshly-measured border-box width of the position reference element (the Form-Field-Wrapper). Moving width here eliminates the stale state entirely, using floating-ui's own recommended pattern for this purpose. It also removes the need for the menuWidth state variable and all associated code in Autocomplete.rebuilt.tsx.
Updating the visual regression snapshots
clientWidth is a DOM property that returns a rounded integer representing the element's content + padding width, explicitly excluding the border. Meanwhile,
rects.reference.widthcomes from floating-ui's internal call togetBoundingClientRect().width. That returns a floating-point number representing the element's border-box width: content + padding + border.The outcome of this is that the menu is 2px wider on both sides to include the border width. This is most obvious by using the onion view when inspecting the screenshots. As you slide from left to right you will see the menu increase in width slightly as it shifts from the old styles to the new styles. Otherwise, everything else should be functionally the same.
Testing
The component should look and behave the exact same as before except for no longer crashing on resize with the menu open.
Changes can be
tested via Pre-release
In Atlantis we use Github's built in pull request reviews.